Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[로또 미션] 김성재 미션 제출합니다. #47

Open
wants to merge 6 commits into
base: seongjae6751
Choose a base branch
from

Conversation

seongjae6751
Copy link

여러 날에 걸쳐서 충분히 고민해가보며 했어야 했는데 급하게 몰아서 했네요..ㅠ
상당히 코드가 정돈 되지 못한 것 같은데 어떻게 해야 더 나을지 계속 고민해보겠습니다.

Copy link

@dradnats1012 dradnats1012 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다!!!~!

import domain.LottoMachine;
import domain.WinningNumbers;

public class Application {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

개인적으로 실행시키는 클래스는 깔끔해야 한다고 생각해서 동작들을 클래스로 빼면 좋을 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

타당한 말씀이십니다! 👍

Comment on lines 18 to 19
int manualCount = scanner.nextInt();
scanner.nextLine();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

int manualCount = Integer.parseInt(scanner.nextLine);
으로 한번에 하는건 어떤가요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

꼼꼼하시네요

System.out.println("수동으로 구매할 번호를 입력해 주세요.");
for (int i = 0; i < manualCount; i++) {
String input = scanner.nextLine();
String[] inputs = input.split(", ");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이러면 공백이 없을 경우에 오류가 날수도 있을 것 같아요

Comment on lines +18 to +20
if (this.numbers.size() != 6) {
throw new IllegalArgumentException("로또 번호는 중복되지 않아야 합니다.");
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

로또 번호를 만들때 중복검사하는건 어떤가요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

현재 set으로 중복 검사하고 있습니다!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

InputHandler 클래스인만큼 입력값만 받아주고 처리는 해당 입력을 사용하는 클래스에서 하는게 좋다고 생각하는데 어떻게 생각하시나요?

Comment on lines 31 to 49
private WinningRank determineRank(Lotto lotto) {
int match = winningNumbers.countMatchingNumbers(lotto);
if (match == 6) {
return WinningRank.SIX;
}
if (match == 5 && winningNumbers.containsBonusNumber(lotto)) {
return WinningRank.FIVE_WITH_BONUS;
}
if (match == 5) {
return WinningRank.FIVE;
}
if (match == 4) {
return WinningRank.FOUR;
}
if (match == 3) {
return WinningRank.THREE;
}
return WinningRank.NONE;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WinningRank 클래스에 bonusNumber 여부도 추가해서 관리하는것도 괜찮을 것 같아요

Comment on lines 52 to 60
System.out.println("당첨 통계");
System.out.println("---------");
System.out.println("3개 일치 (5000원) - " + matchCount.get(WinningRank.THREE) + "개");
System.out.println("4개 일치 (50000원) - " + matchCount.get(WinningRank.FOUR) + "개");
System.out.println("5개 일치 (1500000원) - " + matchCount.get(WinningRank.FIVE) + "개");
System.out.println("5개 일치, 보너스 볼 일치 (30000000원) - " + matchCount.get(WinningRank.FIVE_WITH_BONUS) + "개");
System.out.println("6개 일치 (2000000000원) - " + matchCount.get(WinningRank.SIX) + "개");
double profitRate = calculateProfitRate(matchCount);
System.out.printf("총 수익률은 %.4f입니다.%n", profitRate);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이렇게 하면 변경이 일어날경우 고치기 힘들 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋은 말씀이네용 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants